fix(formatNumber): allow negative fraction size #13913
Conversation
| } else { | ||
| // We rounded to zero so reset the parsedNumber | ||
| parsedNumber.i = 1; | ||
| digits.length = roundAt = fractionSize + 1; | ||
| for (var i=0; i < roundAt; i++) digits[i] = 0; | ||
| digits.length = Math.max(1, roundAt = fractionSize + 1); |
There was a problem hiding this comment.
AFAICT, fractionSize + 1 can never be > 1, because:
fractionSize + 1 > 1 <=>
fractionSize > 0 <=>
fractionSize + parsedNumber.i > 0 (because `parsedNumber.i > 0`) <=>
roundedAt > 0 (on line 220) <=>
We wouldn't be on this branch of the if-else block
There was a problem hiding this comment.
Hm...it seems that parsedNumber.i can be < 1, so ignore the above :)
|
Other than the 2 issues pointed in the comments, it LGTM. |
jbedard
force-pushed
the
negative-precision
branch
2 times, most recently
from
43d5480 to
eb38484
Compare
|
Added tests and your fixes. Thanks for showing the exact fixes! |
| digits.splice(Math.max(parsedNumber.i, roundAt)); | ||
|
|
||
| // Set non-fractional digits beyond `roundAt` to 0 | ||
| for (var j=roundAt; j < digits.length; j++) { |
There was a problem hiding this comment.
Interesting that JSCS doesn't complain about no space around = (I thought it would) :)
There was a problem hiding this comment.
Updated! Line 237 also updated, note that that one had no spaces before this commit as well...
There was a problem hiding this comment.
Yeah, I didn't mean you had to change it necessarily - I was just surprised with JSCS.
But look how much better it looks now 😛
|
LGTM |
jbedard
force-pushed
the
negative-precision
branch
from
eb38484 to
aeb0d01
Compare
|
@gkalpak do you want to merge? |
|
@Narretz, on it ! Thx for the reminder 😃 |
|
Backported to |
This worked before 6a0686d. The other option would be to not support this and just convert
fractionSize < 0to0.This solution could probably be improved though...